Skip to content

fix(security): rotation JWT compliance (#25, #26)#39

Merged
moisesja merged 3 commits into
mainfrom
feat/security-rotation-compliance
Jun 17, 2026
Merged

fix(security): rotation JWT compliance (#25, #26)#39
moisesja merged 3 commits into
mainfrom
feat/security-rotation-compliance

Conversation

@moisesja

@moisesja moisesja commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Summary

Resolves two Low-severity audit findings on the from_prior rotation path (#25, #26). Both passed a PoC-backed adversarial red-team pass (AGENTS.md §2). Full suite: 625 tests green; warnings-as-errors clean.

Fixes

# Change
#25 FromPriorBuilder.BuildAsync previously serialized only {iat, iss, sub}, silently discarding FromPriorClaims.Exp/Nbf — so every self-issued rotation JWT was non-expiring and the validator-side FR-ROT-05 freshness control could never fire. Claims are now built via an insertion-ordered JsonObject (exp, iat, iss, nbf, sub), emitting exp/nbf only when set (no-expiry payload byte-identical). New BuildAsync(claims, signerJwk, TimeSpan lifetime) overload sets exp = iat + lifetime so issuing a freshness-bounded token is one call.
#26 FromPriorValidator hand-parses the JWS header and previously read only alg/kid, silently ignoring crit — unlike JwsParser/JweParser, which fail closed (RFC 7515 §4.1.11). It now rejects any crit member with ProtocolException, before signature verification.

The validator/enforcement side already worked (ValidateFromPriorFreshness enforces exp/nbf with clock skew) — so these are narrow, sender-side and header-parse gaps.

Decision applied

The PRD's rotation cookbook (§N) and API matrix referenced a .WithDidRotation(...) helper that was never built (the real API is FromPriorBuilder.BuildAsync + MessageBuilder.WithFromPrior). Per the maintainer's call, the PRD example is realigned to the actual API; WithDidRotation is noted as possible future DX rather than built.

Adversarial red-team pass (AGENTS.md §2) — both hold

Test plan

Closes #25, closes #26

🤖 Generated with Claude Code

@moisesja @claude
fix(security): rotation JWT compliance (#25, #26)
d436756 
Resolves two Low-severity audit findings on the from_prior rotation path.
Both passed a PoC-backed adversarial red-team pass (AGENTS.md §2). Full
suite 625 green; warnings-as-errors clean.

- #25 FromPriorBuilder now emits exp/nbf (FR-ROT-05). It previously serialized
  only {iat,iss,sub}, discarding FromPriorClaims.Exp/Nbf, so every self-issued
  rotation JWT was non-expiring and the validator-side freshness control could
  never fire. Claims are built via an insertion-ordered JsonObject
  (exp,iat,iss,nbf,sub), emitting exp/nbf only when set (no-expiry payload is
  byte-identical). New BuildAsync(claims, signerJwk, TimeSpan lifetime) overload
  sets exp = iat + lifetime (rejects a sub-second lifetime — red-team: avoids a
  silent exp==iat already-expired token). PRD rotation cookbook/API-matrix
  realigned to the real API (BuildAsync + WithFromPrior); the never-built
  WithDidRotation helper is noted as future DX.
- #26 from_prior validator now rejects a crit protected header (RFC 7515
  §4.1.11) with ProtocolException, before signature verification — parity with
  JwsParser/JweParser, which previously the hand-rolled from_prior parse omitted.

Closes #25, closes #26

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@moisesja moisesja self-assigned this Jun 17, 2026
@moisesja moisesja added this to the 1.0.0 GA milestone Jun 17, 2026
moisesja and others added 2 commits June 17, 2026 00:05
@moisesja @claude
test(rotation): add symmetric Nbf future-dated rejection (PR #39 review)
765b91f 
The plan promised an end-to-end test for a future nbf being rejected on
unpack (FR-ROT-05); only the expired-Exp case was present. Adds
DidCommClient_RejectsNotYetValidFromPrior_FrRot05: pins the clock to iat so
nbf = iat + 1 day is deterministically not-yet-valid (the literal "Nbf = iat +
86400" with the 2023 iat would otherwise be in the past under the real clock),
packs authcrypted, and asserts UnpackAsync throws ConsistencyException
("not yet valid"). Full suite 626 green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@moisesja
No branch names on log
486d36e
@moisesja

moisesja commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

Good catch — added in 765b91f: DidCommClient_RejectsNotYetValidFromPrior_FrRot05, the symmetric end-to-end test for a future nbf.

One subtlety worth flagging on the suggested Nbf = iat + 86400: with the fixture iat = 1700000000 (Nov 2023), iat + 86400 is still 2023 — i.e. in the past under the real wall clock, so the not-yet-valid check wouldn't fire (the token would be valid now). To make it both faithful to the intent and deterministic, the test pins the clock to iat via DidCommOptions.Clock, so nbf = iat + 1 day is genuinely in the future (well past any skew) regardless of when CI runs:

var pinnedNow = DateTimeOffset.FromUnixTimeSeconds(iat);
var claims = new FromPriorClaims(Sub: "did:example:alice", Iss: PriorDid, Iat: iat, Nbf: iat + 86400);
// … pack authcrypted …
var client = new DidCommClient(secrets, keyService, new DidCommOptions { Clock = () => pinnedNow });
await act.Should().ThrowAsync<ConsistencyException>().Where(e => e.Message.Contains("not yet valid"));

Both freshness directions are now covered end-to-end (expired exp and future nbf). Full suite 626 green.

@moisesja moisesja merged commit fb55aab into main Jun 17, 2026
2 checks passed
@moisesja moisesja deleted the feat/security-rotation-compliance branch June 17, 2026 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment